Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add lshift and rshift operators #7741

Merged
merged 21 commits into from
Apr 28, 2023
Merged

Add lshift and rshift operators #7741

merged 21 commits into from
Apr 28, 2023

Conversation

abrammer
Copy link
Contributor

@abrammer abrammer commented Apr 8, 2023 •
edited
Loading

This seems a little too easy, but the PR just adds lshift and rshit to generate_ops.py and ops.py then generated the 2 stub files with generate_ops.py.

  • Closes Implement lshift and rshift operators #7727
  • Tests added for:
    • xr.Variable
    • DataArray
    • dask
    • groupby
  • User visible changes (including notable bug fixes) are documented in whats-new.rst
    • There will be new << >> syntax for dataarrays and datasets.
  • New functions/methods are listed in api.rst
    • Existing basic math operators aren't really listed, that I can find.
    • Could add example code in the "Data arrays also implement many numpy.ndarray methods:" section of computation.rst ?

I'll wait for initial review before spending time on whats-new and api docs.

This enables bit shifting on arrays (when the type supports it).

import xarray as xr

x = xr.DataArray(5)
print(x << 1)
# <xarray.DataArray ()>
# array(10)


x = xr.DataArray([0,1,2,3,4,5])
print(x << 1)
# <xarray.DataArray (dim_0: 6)>
# array([ 0,  2,  4,  6,  8, 10])
# Dimensions without coordinates: dim_0


x = xr.DataArray([0,1,2,3,4,5])
print(x >> 1)
# <xarray.DataArray (dim_0: 6)>
# array([0, 0, 1, 1, 2, 2])
# Dimensions without coordinates: dim_0


x = xr.DataArray([0,1,2,3,4,5.])
print(x >> 1)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/workspaces/xarray/xarray/core/_typed_ops.py", line 255, in __rshift__
    return self._binary_op(other, operator.rshift)
  File "/workspaces/xarray/xarray/core/dataarray.py", line 4566, in _binary_op
    f(self.variable, other_variable)
  File "/workspaces/xarray/xarray/core/_typed_ops.py", line 461, in __rshift__
    return self._binary_op(other, operator.rshift)
  File "/workspaces/xarray/xarray/core/variable.py", line 2662, in _binary_op
    f(self_data, other_data) if not reflexive else f(other_data, self_data)
TypeError: ufunc 'right_shift' not supported for the input types, and the inputs could not be safely coerced to any supported types according to the casting rule ''safe''

@abrammer abrammer changed the title Feat/add lshift and rshift operators Add lshift and rshift operators Apr 8, 2023
@dcherian
Copy link
Contributor

I don't see tests for other ops. Are these tested somewhere if so I can add tests when I find them.

Grepping for unary_op and binary_op shows a bunch in test_dataset.py, test_dataarray.py, test_variable.py

@dcherian
Copy link
Contributor

I forgot to say, this looks pretty great already. We just need tests.

Thank you!

Illviljan
Illviljan reviewed Apr 21, 2023
View reviewed changes
xarray/tests/test_dask.py Outdated Show resolved Hide resolved
xarray/tests/test_variable.py Outdated Show resolved Hide resolved
abrammer and others added 3 commits April 21, 2023 12:19
@abrammer @Illviljan
Apply typing suggestions from code review
9961aa5 
Co-authored-by: Illviljan <14371165+Illviljan@users.noreply.github.com>
@abrammer
Fix type checking on test_dask addition
13193bc
@abrammer
Remove new type checking on test_variable edits
e35c22f 
Type checking throws errors on existing test lines
@Illviljan Illviljan mentioned this pull request Apr 23, 2023
Closed
4 tasks
dcherian
dcherian approved these changes Apr 24, 2023
View reviewed changes
Copy link
Contributor

@dcherian dcherian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks @abrammer !

@abrammer
Copy link
Contributor Author

The commits yesterday were to add an entry to whats-new and a couple examples lines to the computation doc page. I didn't find the binary_ops listed in methods anywhere, so this was the best idea I had? In the block just above missing-values: https://xray--7741.org.readthedocs.build/en/7741/user-guide/computation.html#missing-values

Otherwise, I think this is done from my perspective.

@headtr1ck headtr1ck added enhancement plan to merge Final call for comments labels Apr 27, 2023
@dcherian
Copy link
Contributor

Thanks @abrammer very nice work!

@dcherian dcherian merged commit a220022 into pydata:main Apr 28, 2023
dcherian added a commit to dcherian/xarray that referenced this pull request May 6, 2023
@dcherian
Merge branch 'main' into speedup-dt-accesor
993ad42 
* main:
  Introduce Grouper objects internally (pydata#7561)
  [skip-ci] Add cftime groupby, resample benchmarks (pydata#7795)
  Fix groupby binary ops when grouped array is subset relative to other (pydata#7798)
  adjust the deprecation policy for python (pydata#7793)
  [pre-commit.ci] pre-commit autoupdate (pydata#7803)
  Allow the label run-upstream to run upstream CI (pydata#7787)
  Update asv links in contributing guide (pydata#7801)
  Implement DataArray.to_dask_dataframe() (pydata#7635)
  `ds.to_dict` with data as arrays, not lists (pydata#7739)
  Add lshift and rshift operators (pydata#7741)
  Use canonical name for set_horizonalalignment over alias set_ha (pydata#7786)
  Remove pandas<2 pin (pydata#7785)
  [pre-commit.ci] pre-commit autoupdate (pydata#7783)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Illviljan Illviljan Illviljan left review comments

@dcherian dcherian dcherian approved these changes

Assignees
No one assigned
Labels
enhancement plan to merge Final call for comments
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Implement lshift and rshift operators
4 participants
@abrammer @dcherian @Illviljan @headtr1ck